Skip to content

Fix flaky test #1142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Fix flaky test #1142

wants to merge 2 commits into from

Conversation

@penge
Copy link

@penge penge commented Apr 4, 2020

Fixes flaky 'Sniff interval' in sniff.test.js.

The problem was caused by not waiting for the asynchronous kill() to finish the task,
before making a request about the current connectionPool.size.

The error was:

t.strictEqual(client.connectionPool.size, 3)

// expected: 3  => because one node should be killed
//   actual: 4  => because the request was made before the node was killed

The solution was to update kill() by adding a callback and calling it after the stop() in stoppable.js has finished. This was then used in sniff.test.js and resurrect.test.js.

Logging in shutdown() is also improved.

Unused spawn() is removed.

Closes #1108.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Apr 4, 2020

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cla-checker-service
Copy link

cla-checker-service bot commented Apr 4, 2020 •
edited
Loading

💚 CLA has been signed

@penge
Copy link
Author

penge commented Apr 4, 2020

Here it is @delvedor ;)

@penge
Copy link
Author

penge commented Apr 5, 2020

I've added some more setTimeout calls to test that the SNIFF event is only run when:

1) delay is greater than sniffInterval
2) delay is greater than time of last sniff + sniffInterval

This is also mentioned in the added comments.

I think it's now good to merge.

delvedor
delvedor reviewed Apr 6, 2020
View reviewed changes
@delvedor delvedor added the tests label Apr 6, 2020
@penge penge requested a review from delvedor April 7, 2020 13:59
@penge penge mentioned this pull request Apr 13, 2020
Closed
penge added 2 commits April 13, 2020 16:36
@penge
Fix flaky test
9ff031d 
The problem was caused by not waiting
for the asynchronous kill() to finish,
before making a request to get
connectionPool.size
@penge
Check if conn exists before marking it dead/alive
b070706
@penge penge mentioned this pull request Apr 13, 2020
Closed
@delvedor delvedor mentioned this pull request Apr 14, 2020
Merged
delvedor
delvedor reviewed Apr 14, 2020
View reviewed changes
Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! I'm sorry but there are too many changes in this pr not related to the issues, making it very hard to review.
I've opened #1158, which should fix the issues once for all.
Thank you for contributing!

@delvedor delvedor closed this Apr 14, 2020
@penge
Copy link
Author

penge commented Apr 14, 2020

No problem. Cool

@penge penge deleted the pb-fix-flaky-test branch April 14, 2020 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@delvedor delvedor delvedor left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix flaky test

3 participants

@penge @elasticmachine @delvedor